Do we need to check for "MultiThreadedDebugDLL" specifically? I think the libc++ logic looks appropriate to use here. See

I know we expect CMAKE_MSVC_RUNTIME_LIBRARY to always be set currently, but this logic will still work as expected if we relax that restriction later.

we don't expect it to be always set. We are ok with cmake default value MultiThreaded$<$CONFIG:Debug:Debug>DLL and handle explicitly set values if needed.

MultiThreadedDebugDLL is the only option that is supported for sycl in Debug mode. Why should we introduce confusion by setting "CMAKE_MSVC_RUNTIME_LIBRARY MATCHES "Debug""?

I updated this code but left check of full config name "MultiThreadedDebugDLL".
I know that we do a check for "DLL" match a few lines before. Although I want to keep the full name if it is the only option there.
e502190

Hmm, I didn't realize that MultiThreadedDebugDLL was the only supported option when building in debug mode. It seems that should be asserted rather than result in the "d" suffix being silently dropped. Should we also ensure that a MT variant is used? How about this?

  if (CMAKE_MSVC_RUNTIME_LIBRARY
      AND (NOT CMAKE_MSVC_RUNTIME_LIBRARY MATCHES "^MultiThreaded.*DLL$"))
    message(FATAL_ERROR "libsycl requires a multi-threaded DLL version of the MSVC CRT.")
  endif()
  if (uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
    if (NOT CMAKE_MSVC_RUNTIME_LIBRARY STREQUAL "MultiThreadedDebugDLL")
      message(FATAL_ERROR "Debug builds of libsycl require a multi-threaded debug DLL version of the MSVC CRT.")
    endif()
    set(LIB_OUTPUT_NAME "${LIB_OUTPUT_NAME}d")
  endif()

I think in our case check for DLL and MultiThreaded*DLL does completely the same. All options start from "MultiThreaded", this change of check brings nothing.
options are:
MultiThreaded
MultiThreadedDLL
MultiThreadedDebug
MultiThreadedDebugDLL

so with the current impl:
If user sets MultiThreaded, MultiThreadedDebug - user receives error "libsycl requires a DLL version of the MSVC CRT".
Other two options are valid and lead to setting "d" suffix or not.

Thanks, I forgot that the DLL variants are always multi-threaded.

Hmm, CMake doesn't seem to acknowledge the existence of the single-threaded static CRT library (other than to disable automatically linking with it for at least some targets; https://github.com/Kitware/CMake/blob/master/Modules/Platform/Windows-MSVC.cmake#L203-L205).

The intent is that the "d" suffix is appended if, and only if, the target is linked with a debug version of the MSVC CRT, correct? I had a different understanding of the intent in earlier comments which is why my last suggestion limited appending the "d" suffix to debug builds.

You previously stated:

MultiThreadedDebugDLL is the only option that is supported for sycl in Debug mode.
If the intent is that debug builds of libsycl are always required to link with the (multi-threaded) debug (DLL) variant of the MSVC CRT, then I think a change is warranted to assert that. However, inline with my previous statement about the intent being that "d" is only appended when linking with a debug MSVC CRT variant, then debug builds of libsycl may link with the (multi-threaded) release (DLL) variant of the MSVC CRT. In that case, the code is fine as is.

I still have a slight preference for just checking for a match of "Debug" since I think that best expresses the intent and is consistent with libc++, but I acknowledge that it doesn't change the result. So, your choice how you want to handle this.

"The intent is that the "d" suffix is appended if, and only if, the target is linked with a debug version of the MSVC CRT, correct?"
yes, it is how we do it now.
I would prefer to leave full name (MultiThreadedDebugDLL) since it represents better that SYCL RT is always a DLL, there is no other options.

The required use of a DLL is already assured at this point.

My concern is the "MultiThreaded" part and exact matching. Though CMake doesn't currently acknowledge it, there are additional MSVC CRT implementations; see the MSVC /Qspectre option and the special support for it that exists in MSVS for C++ projects via the "Spectre Mitigation" property. I think it isn't unreasonable that CMake might expose these additional run-time libraries; particularly for the MSBuild/Visual Studio generator. I don't know if the spectre CRT libraries are provided in debug form, but I would rather err on the side of adding the "d" suffix when "Debug" is present in the name than risk not adding it when libsycl is linked with an unknown CRT library.

4c17316

4c17316

4c17316